Skip to content

refactor(solana): move message encoding to shared Kotlin - #1456

Open
bmc08gt wants to merge 26 commits into
code/cashfrom
refactor/solana-encoding-kmp-prep
Open

bmc08gt wants to merge 26 commits into
code/cashfrom
refactor/solana-encoding-kmp-prep

Conversation

@bmc08gt

@bmc08gt bmc08gt commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

The Solana message and transaction encoding layer becomes shared Kotlin, and the two canonical
vector suites that guard it now run on the Apple targets instead of only the JVM. That last part is
the point: solana_message.json and compact_message.json passing on iosSimulatorArm64 and
macosArm64 is what proves the Kotlin produces the same bytes the Swift implementation does.

This is slice 1 of C3 in docs/shared-reality-milestones.md, scoped to encoding alone. Transaction
building, program instructions, and intent construction are not in it.

What moved

:libs:encryption:keys becomes a KMP module, and a new :libs:solana:encoding holds nine files —
Message, LegacyMessage, VersionedMessage, MessageHeader, Instruction, SolanaTransaction,
ShortVec, MessageAddressLookupTable, AddressLookupTable — with packages unchanged, so
:services:opencode consumers resolve as before through an api(...) dependency.

Five test classes move to commonTest with them. :services:opencode:test goes from 51 test
classes to 46; the five reappear on the JVM host, iosSimulatorArm64, and macosArm64.

Things worth a reviewer's attention

Parcelable became @TypeParceler. An androidMain source set cannot add a supertype to a
commonMain class, so PublicKey and Mint lose their Parcelable conformance and get
Parcelers in androidMain, applied at 19 @TypeParceler annotations across 9 holder classes.
The parcel format is unchanged — both parcelers write the same base58 string the removed
writeToParcel did, .orEmpty() included. Nothing in this repo tests parcel round-trips, so that
claim rests on reading the two implementations side by side.

Signing did not move. SolanaTransaction.sign/signatures depend on com.getcode.ed25519.Ed25519
— the JNI wrapper in :libs:encryption:ed25519-native, not the KMP Ed25519Kmp — which kept the
module from compiling on Apple at all. They are now extension functions in :services:opencode,
moved verbatim, alongside diff() and fromBytes() which sit there for the same reason. Migrating
the app off the JNI Ed25519 touches 138 files and belongs to Track B.

:libs:encryption:keys was missing from kmpUnitTestModules. Converting it in place left it in
androidUnitTestModules, where CI would have called testDebugUnitTest — a task the conversion
removes. Fixed in settings.gradle.kts.

The shared-core export is wider than the facade. SolanaEncoding is a flat ByteArray-in/out
entry point and SharedCoreKit's SolanaEncoding.swift names no Kotlin type, but export(project(...))
exports a module's whole public API — so the framework header also carries Message, Instruction,
SolanaTransaction, AddressLookupTable, MessageHeader, AccountMeta, PublicKey, and Mint.
Each collides with a type of the same name in FlipcashCore. It only bites a Swift file that imports
SharedCore directly alongside FlipcashCore; iOS adoption goes through SharedCoreKit, so it
stays contained. Narrowing it means making those types internal, and they are public API to 40+
files in :services:opencode — a separate change.

Not in this PR

No SharedCore version is published. iOS adoption develops against FLIPCASH_SHARED_CORE_LOCAL and
ships separately, so no version number is spent until a build outside this repo needs one.

javaClass does not compile in a commonMain source set. this::class/other::class
is the multiplatform-safe equivalent and preserves the exact-type semantics:
same bytes across two different KeyType subclasses (e.g. Key64 vs Signature)
still compare unequal.

Adds a KeyTest case for that cross-subclass comparison, which the existing
suite exercised implicitly but never asserted.
DataSlice was internal to :services:opencode, so extracting the Solana
encoding layer in a later phase would break the six call sites outside that
package (ComputeBudgetProgram_*, InstructionType, SwapValidatorProgram,
TimelockProgram, OpenCodePayload, PayloadKind). It has no Solana-specific
logic — generic byte-list slicing — so it moves to commonMain in
:libs:encryption:utils, which already builds for Android and all five Apple
targets, and becomes a public object. :services:opencode already depended on
that module, so no build.gradle.kts change was needed.
java.io.ByteArrayInputStream does not compile in commonMain. decodeLen already
reads one byte at a time and stops on the continuation bit, so it walks the
input List<Byte> with an index instead of wrapping it in a stream. Same
byte-for-byte decode.
trace() reaches :libs:logging, which is Android-only. The ten calls in
VersionedMessageV0.newInstance were decode-path diagnostics only — none of
them affect control flow, every one sits right next to a `return null` that
fires with or without it. Removed rather than replaced, since neither of them
guards a branch a caller could act on.
…ion types

com.google.protobuf.ByteString is a proto-boundary type; the classes that will
move to commonMain (Signature, PublicKey, SolanaTransaction) shouldn't import
it directly. Moved each ByteString entry point into its own file next to the
type it serves:

- Signature's `ByteString` constructor becomes a top-level pseudo-constructor
  function in ByteStringKeys.kt. Kotlin resolves a top-level function sharing
  a class's name alongside its real constructors, so `Signature(byteString)`
  call sites keep compiling unchanged.
- PublicKey.fromByteString becomes an extension on PublicKey.Companion in the
  same file — no call sites exist for it today, but the call syntax
  `PublicKey.fromByteString(...)` would still resolve if one existed.
- SolanaTransaction.fromBytes becomes an extension on
  SolanaTransaction.Companion in ByteStringSolanaTransaction.kt. Its one call
  site (StatefulSwapExecutor.kt) needed an added import for the now-external
  function, since extension functions aren't pulled in by importing the class
  they extend.

libs:encryption:keys:test and services:opencode:test both pass, including
SolanaMessageVectorTest and CompactMessageVectorTest.
Move src/main/kotlin to commonMain (packages unchanged) and split
Android-only pieces (ByteListSerializer, ByteStringKeys) into androidMain,
so the module builds for android, iosArm64/iosX64/iosSimulatorArm64, and
macosArm64/macosX64 ahead of the Solana-encoding sharing work.

The bespoke build.gradle.kts replaces flipcash.android.library, so it has
to explicitly apply org.jetbrains.kotlin.plugin.serialization: the old
convention plugin applied it automatically, and without it @serializable
codegen for Mint/PublicKey silently falls back to reflection and throws at
runtime.

grpc-okhttp and grpc-kotlin were dependencies of the old module but nothing
under src/ references gRPC; dropped rather than carried into the split.

KeyType.base64()/base64Redacted() depended on the Android-only
com.getcode.utils.encodeBase64, so they move to a new androidMain
KeyBase64.kt. No production code calls them today, so this is a relocation
rather than a behaviour change.
PublicKey and Mint hand-rolled android.os.Parcelable (writeToParcel,
CREATOR) directly in commonMain, which does not compile outside
androidMain. Move that to two Parceler objects in a new androidMain
KeyParcelers.kt (PublicKeyParceler, MintParceler) and reference them from
every holder via kotlinx.parcelize.TypeParceler, so the wire format is
unchanged: still a single writeString/readString of the base58 address.

TypeParceler only applies at class or property targets, not file, so it is
added directly on each concrete data class that carries a Mint or PublicKey
field, including classes that inherit their Parcelize codegen from a parent
sealed interface (e.g. DeeplinkType.TokenInfo) rather than declaring their
own Parcelize.

Nine holders needed it, not the ~11 the plan estimated:
- apps/flipcash/core: AppRoute (Give, Info, Transactions, Withdrawal),
  DepositStep.Destination, WithdrawalStep.Amount, DeeplinkType.TokenInfo,
  WalletDeeplinkConnectionResult.ExternalWalletConnection,
  TokenPurpose (Swap, ConvertDestination, BuyFunding),
  TokenSwapPurpose (Buy, Sell, Convert)
- services/opencode: MintMetadata (MintMetadata, VmMetadata,
  LaunchpadMetadata), LocalFiat

VerifiedFiatCalculator's VerifiedFiat and SwapId do not carry a
Mint/PublicKey field directly (VerifiedFiat only parcels its nested
LocalFiat; SwapId's PublicKey is a derived property, not a stored one), so
neither needs its own annotation.
KeyTest, MintTest, PublicKeyTest, and AccountMetaTest only use kotlin.test
assertions, so they move to commonTest and now run against every KMP
target. SerializerTest stays JVM-only in androidHostTest since it drives
Robolectric (RobolectricTestRunner, Config.NONE) to exercise the
ByteListAsBase64Serializer/PublicKeyAsStringSerializer JSON round trip.
Add the module (Android + iosArm64/iosSimulatorArm64/iosX64/macosArm64/
macosX64, matching :libs:currency-math:discrete-curve) and register it in
settings.gradle.kts, including kmpUnitTestModules so its aggregate test task
picks it up.
Move Instruction, Message, MessageHeader, LegacyMessage, VersionedMessage,
SolanaTransaction, ShortVec, MessageAddressLookupTable, and
AddressLookupTable out of :services:opencode and into the new module's
commonMain, keeping their packages. ShortVec stays internal, so
ShortVecTest.kt moves with it — internal visibility is module-scoped and
the test can no longer compile against :services:opencode once the type
lives elsewhere.

Extract SolanaTransaction.diff() into SolanaTransactionDiff.kt, staying in
:services:opencode: it depends on Differ.kt's printDiff/printMatch, which
use timber.log.Timber and can't move. Its MessageHeader.description
reference is inlined rather than exposed, since that property is internal
inside the new module and diff() was its only caller.

Replace the two javaClass-based equals() checks (Instruction, MessageHeader)
with `other !is X`, matching the KMP-safe pattern already used elsewhere
(Key32, PublicKey) — javaClass doesn't resolve on Kotlin/Native. Neither
class has subclasses, so this preserves current equals() semantics exactly.
Add it as api so transitive consumers of :services:opencode
(PhantomWalletController et al.) keep resolving Instruction, Message,
SolanaTransaction, and friends without adding their own dependency now
that those types live in a separate module.
IntentExecutor and StatefulSwapExecutor call SolanaTransaction.diff() to
log what differs when the server rejects a transaction for an invalid
signature - a real call site, not the dead code the extraction assumed.
Update both imports to the function's new package and correct the doc
comment that said otherwise.
SolanaTransaction.sign/signatures depended on com.getcode.ed25519.Ed25519,
the Android-only JNI wrapper (its KeyPair is Parcelable), not the KMP
Ed25519Kmp. That import kept :libs:solana:encoding from compiling on the
Apple targets, since the module is meant to be encoding only.

Move both functions to services/opencode/src/main/kotlin/com/getcode/opencode/solana/SolanaTransactionSigning.kt
as extensions on SolanaTransaction, unchanged, alongside SigningError. Ed25519
stays the legacy JNI class; nothing migrates to Ed25519Kmp here.
Move the Solana legacy-message and compact-message vector tests (plus
VersionedMessageV0Test, SolanaTransactionLookupTableTest, and
InstructionIntegrationTest) from services/opencode/src/test into
libs/solana/encoding/src/commonTest, alongside the encoding code they
exercise. Neither moved test calls the signing extension that still
lives in :services:opencode, so nothing needs to stay behind.

Wire the flipcash.kmp.test.fixtures plugin so the two vector JSON
files compile into a generated TestFixtures.kt readTestResource(),
since Kotlin/Native test binaries carry no resource bundle. Replace
the JVM-only bits the moved tests relied on (javaClass.getResourceAsStream,
String.format("%02x"), toByteArray(Charsets.UTF_8), java.security.MessageDigest)
with portable equivalents (readTestResource, hexEncodedString,
encodeToByteArray, Sha256Hash) without changing what either test
asserts.

Update test-vectors/README.md's run matrix and sync commands to point
at the new location and note both suites now also run on
iosSimulatorArm64Test/macosArm64Test, not just the JVM host.
:libs:encryption:keys became a KMP module (com.android.kotlin.multiplatform.library
+ withHostTest) earlier on this branch but was never added to
kmpUnitTestModules, so it fell into androidUnitTestModules and the
flipcashTestDebug aggregate would have invoked the nonexistent
testDebugUnitTest task for it instead of testAndroidHostTest.

Add it to kmpUnitTestModules, ordered after its dependencies
(base58, sha256, utils) and before :libs:solana:encoding, which
depends on it.
…try point

Add :libs:solana:encoding and :libs:encryption:keys to shared-core's
export/api blocks so the Obj-C framework carries Solana wire-format
encode/decode.

Exporting the module surfaces its whole public API, including the
Message/SolanaTransaction/Instruction/AddressLookupTable hierarchy the
Swift FlipcashCore side already defines under the same names. Add
SolanaEncoding as the entry point callers actually use: four
ByteArray-in/ByteArray-out functions (decode/encode message,
decode/encode transaction) that never take or return a Message or
SolanaTransaction, so the facade layer never needs the leaked types.
Wrap SharedCore.SolanaEncoding in a Data-in/Data-out SharedSolanaEncoding
enum, matching the style of Base58.swift and Derivation.swift. Callers
never touch the underlying Kotlin Message/SolanaTransaction types.

Copy solana_message.json and compact_message.json from test-vectors/
and assert the same vectors the Kotlin commonTest suite does:
decodeMessage/encodeMessage round-trip and mutate the canonical legacy
messages byte for byte, and the compact-message byte composition and
its SHA-256 are replicated directly since SharedHash.sha256 already
crosses the bridge. Transaction encode/decode is exercised against
transactions built from those same message vectors, since no canonical
fixture covers full transactions yet.
@bmc08gt bmc08gt self-assigned this Sep 12, 2026
@github-actions github-actions Bot added area: crypto Solana, keys, encryption, signing type: refactor Code restructuring, no behavior change area: network gRPC, connectivity, API, exchange rates area: build-system Gradle, convention plugins, build-logic area: tokens Token accounts, balances, token info area: intents Intent construction, submission, server-side state labels Sep 12, 2026
SharedCoreKit's Solana surface was flat Data-in/Data-out only
(SolanaEncoding.swift), enough to round-trip bytes but not to build or
inspect a message. FlipcashCore needs to construct transactions field
by field (accounts, header, instructions, address-table lookups) to
eventually delegate to this framework instead of its own parallel
Swift implementation.

Add SharedSolanaMessage (a value-type enum standing in for Kotlin's
Message sealed interface, which crosses the Obj-C bridge as a
reference-type protocol, plus SharedSolanaLegacyMessage,
SharedSolanaVersionedMessageV0, SharedSolanaAccountMeta,
SharedSolanaInstruction, SharedSolanaCompiledInstruction,
SharedSolanaMessageAddressTableLookup, SharedSolanaAddressLookupTable,
and SharedSolanaTransaction. Transaction construction delegates to the
exported SolanaTransaction.doNewInstance/doNewV0Instance factories, so
the canonical account-sort and V0 address-lookup-table grouping stays
single-sourced in Kotlin rather than reimplemented in Swift.

This hierarchy is the one part of the exported surface with no
unboxed-ByteArray entry point (KeyType/AccountMeta/Instruction only
take List<Byte>, which bridges as boxed KotlinByte per element), so
KotlinByteList+Bridge.swift adds that boxing/unboxing in one place.

Guard SharedSolanaTransaction.init(data:) against empty input
directly: SolanaTransaction.fromList reads its leading ShortVec length
byte with no bounds check (ShortVec.decodeLen), so empty data crashes
the process instead of returning nil.
EOF
)
…nput

ShortVec.decodeLen read input[offset] in an unbounded loop, throwing
IndexOutOfBoundsException on empty input or a truncated length prefix
whose last byte still had its continuation bit set. On Kotlin/Native
that exception is an uncaught, fatal trap across the Swift interop
boundary rather than a catchable error, reachable from a Phantom
wallet deeplink (PhantomWalletController) or a malformed server
response (IntentExecutor).

decodeLen now returns null instead of throwing: on empty input, on a
length prefix that runs past 5 continuation bytes without terminating,
and on a decoded value that would be negative (a crafted 5th byte can
set Int's sign bit). Every call site already propagates that null
through its own nullable return, so no public signature changes.

Auditing decodeLen's callers turned up two more instances of the same
class of bug, untrusted length prefixes used without validating them
against the remaining bytes:
- MessageHeader.fromList indexes its input unconditionally; both
  LegacyMessage.newInstance and VersionedMessageV0.newInstance now
  check the remaining length before calling it instead of changing its
  signature.
- VersionedMessageV0.newInstance decoded static account keys with
  chunked() plus runCatching { PublicKey(chunk) }, which never throws
  (PublicKey accepts any-length input), so a truncated key list was
  silently accepted rather than rejected. Switched to the module's
  bounds-checked DataSlice.chunk, matching how LegacyMessage and
  SolanaTransaction already decode their fixed-size lists.

Bounds checks against attacker-controlled counts use division
(count > available divided by size) rather than multiplication
(count times size > available), since the multiplication can overflow
Int and wrap into a value that passes the check.

Adds regression coverage for empty input, a lone unterminated
continuation byte, and truncation at each stage of the transaction,
legacy message, v0 message, and compiled instruction decoders.
…tlin* aliases

SolanaMessage.swift and SolanaTransaction.swift spelled out SharedCore.X
at every Kotlin type reference, alongside their own Shared*-prefixed
Swift facade types (SharedSolanaMessage, SharedSolanaInstruction, ...).
The two prefixes read as near-duplicates at a glance, which makes it
easy to misread a Kotlin-side reference as the Swift facade type.

Adds KotlinTypes.swift, declaring an internal typealias Kotlin* for
each SharedCore type these two files touch (AccountMeta,
AddressLookupTable, CompiledInstruction, Instruction, Key32,
LegacyMessage, Message, MessageAddressLookupTable, MessageCompanion,
MessageHeader, MessageLegacy, MessageVersionedV0, PublicKey,
Signature, SolanaTransaction, VersionedMessageV0), then rewrites both
files against those aliases. The aliases stay internal: nothing
outside SharedCoreKit should reference a raw Kotlin type, since the
Shared* facade exists precisely so callers never have to.

KotlinByteList+Bridge.swift needed no change: it references only
KotlinByte, already unqualified. BondingCurve.swift, Base58.swift, and
SharedCoreInfo.swift are untouched. Package.swift is unchanged.

Also drops the guard !data.isEmpty in
SharedSolanaTransaction.init(data:), now that
SolanaTransaction.fromList returns null for empty and truncated input
instead of throwing (see the ShortVec.decodeLen fix). The existing
"SharedSolanaTransaction rejects malformed input" test already covers
Data() and continues to pass with the guard gone, now exercising the
full path down into Kotlin rather than short-circuiting on the Swift
side.

Verified with swift test against a locally reassembled
SharedCore.xcframework (FLIPCASH_SHARED_CORE_LOCAL): all 40 existing
tests pass unedited.
…missing

Instruction.compile used indexOfFirst to resolve each account to its index
in messageAccounts, but indexOfFirst returns -1 on a miss and (-1).toByte()
is 0xFF — a structurally valid but wrong index, so a missing account
silently compiled into a corrupt instruction instead of failing. The three
internal callers can't hit this (each passes an account list built from the
same instructions being compiled), but compile is exported to iOS through
SharedCoreKit, where a caller-supplied account list can miss.

compile now returns CompiledInstruction? and returns null on the first
missing program or account. The three callers (LegacyMessage.encode,
Message.instructions, SolanaTransaction.newV0Instance) keep their existing
non-nullable signatures — changing them would cascade into transaction
building and intent construction in :services:opencode and :apps:flipcash,
which this slice of the KMP encoding work doesn't touch — and convert a
null back to an error() at the point each one's own invariant guarantees
compile can't miss.

SharedSolanaInstruction.compile(messageAccounts:) in the Swift facade is
failable to match, and its one internal caller (SharedSolanaMessage.instructions)
mirrors the same error()-on-proven-unreachable-null shape.
…rence unknown accounts

SharedSolanaLegacyMessage's public memberwise init took accounts and
instructions independently, so a caller could build a message whose
instruction referenced an account absent from accounts. Nothing
enforced that invariant, and both encode() and
SharedSolanaMessage.instructions assumed it held, calling into
Kotlin's LegacyMessage.encode()/Message.instructions, which trap with
an uncaught IllegalStateException across the Kotlin/Native boundary
when it doesn't.

Make the public init failable: it now validates every instruction's
program and account keys against accounts and returns nil otherwise.
The internal init(_ message: KotlinLegacyMessage) stays non-failable,
since Kotlin's own LegacyMessage.newInstance already guarantees the
invariant for decoded values. With construction validated,
SharedSolanaMessage.instructions no longer needs its fatalError guard.

Audited the rest of the facade's public inits for the same hole:
SharedSolanaVersionedMessageV0 stores already-compiled,
index-based instructions and never calls compile(), and
SharedSolanaTransaction's payer-based builder inits derive their
account list from instructions inside Kotlin's
doNewInstance/doNewV0Instance rather than accepting one from the
caller, so neither can reach an inconsistent state this way.
SharedSolanaLegacyMessage's public init validates that every
instruction's accounts exist in the message's own account list, but
the four stored properties were still `var`. A caller could extract
the message, mutate `instructions` directly, and reintroduce the
dangling reference the initializer rejects — reaching the same
Kotlin `error(...)` trap the validation was meant to close.

Make the properties `let` so the invariant holds for the value's
whole lifetime. `SharedSolanaMessage.recentBlockhash`'s setter no
longer mutates a `LegacyMessage` copy in place; it goes through a new
`withRecentBlockhash` helper that reconstructs via an unchecked
internal initializer, since replacing the blockhash can't affect the
accounts/instructions invariant.

The `compile(messageAccounts:)!` in `SharedSolanaMessage.instructions`
was already resting on construction-time validation alone, which this
closes; its comment now cites immutability as the reason `nil` is
unreachable there.
…ecompile

decompile validated only the count of accountIndexes against accounts.size
and then indexed with programIndex/accountIndexes unchecked. Both are u8 on
the wire but were read as signed Bytes: a wire byte of 0xFF decodes to -1,
and LegacyMessage.newInstance's own guard on programIndex is also a signed
comparison that -1 passes, so accounts[-1] throws IndexOutOfBoundsException.
A positive index simply past the end of accounts crashes the same way,
since the count check never looks at any individual index's value.

On Kotlin/Native that exception is an uncaught, fatal trap across the Swift
interop boundary rather than a catchable error, and LegacyMessage.newInstance
is the only call site, decoding bytes straight off the wire.

decompile now reads both indexes unsigned (and 0xFF) and range-checks each
against accounts.size, returning null instead of throwing. The redundant
signed check in LegacyMessage.newInstance is dropped rather than fixed in
place: it only ever rejected values decompile now rejects anyway, and kept
around it would contradict the new unsigned check.

Adds regression coverage for a negative-when-signed program index (0xFF), a
positive out-of-range program index, and an out-of-range account index.
…d identity

SwiftPM derives a path dependency's identity from the directory basename,
so the local-override path pointed at kmp/shared-core/spm while
Code.xcodeproj holds an XCRemoteSwiftPackageReference with identity
flipcash-shared-core-spm that no environment variable can switch. Two
identities, same targets: resolving the Flipcash scheme with
FLIPCASH_SHARED_CORE_LOCAL set failed outright with "multiple similar
targets 'SharedCore', 'SharedCoreKit' appear in package 'spm' and
'flipcash-shared-core-spm'". Renaming the directory to match the published
identity collapses the two into one.
@bmc08gt
bmc08gt requested a review from jeffyanta as a code owner September 12, 2026 16:48
…published identity"

This reverts commit 58f60f3.

The rename existed to give the local package the same SwiftPM identity as the
published one, so the app scheme would stop failing with `multiple similar
targets 'SharedCore', 'SharedCoreKit' appear in package 'spm' and
'flipcash-shared-core-spm'`. The second identity came from an
`XCRemoteSwiftPackageReference` in `Code.xcodeproj` that the app target held
for a single call — `KikCode.svg`, in `TipCardExport.swift`.

Removing that reference on the iOS side fixes the collision at its source and
needs no change here. With this revert applied, the app scheme resolves
`SharedCore` once at `kmp/shared-core/spm` under the override, builds, and still
resolves `0.6.0` from the published URL with the override unset.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: build-system Gradle, convention plugins, build-logic area: crypto Solana, keys, encryption, signing area: intents Intent construction, submission, server-side state area: network gRPC, connectivity, API, exchange rates area: tokens Token accounts, balances, token info type: refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant